d/patches: Never mark rendering differences as accepted, only tolerated
authorSimon McVittie <smcv@debian.org>
Mon, 15 Feb 2021 10:00:03 +0000 (10:00 +0000)
committerSimon McVittie <smcv@debian.org>
Mon, 15 Feb 2021 10:47:51 +0000 (10:47 +0000)
Upstream consider absolutely any difference in rendering to be an error,
so we should always record the diff for inspection.

debian/close-enough.keyfile
debian/patches/reftest_compare_surfaces-Report-how-much-the-images-diffe.patch
debian/patches/reftests-Allow-minor-differences-to-be-tolerated.patch

index f3a214beae992bcabc1c3cac1288956ca3ef6f32..1ae6ff92d9ab1f0bec27a17ce34b1f26447cad50 100644 (file)
@@ -1,5 +1,3 @@
 [reftest]
-accepted-diff-level=1
-accepted-diff-pixels=50
 tolerated-diff-level=10
 tolerated-diff-pixels=100
index 38fecd48d038bc07bbeb5392d089219691ab8b6d..f9c47e1f68571607cc3d522eb525c6bf74478fe0 100644 (file)
@@ -2,10 +2,9 @@ From: Simon McVittie <smcv@debian.org>
 Date: Sat, 13 Feb 2021 18:26:24 +0000
 Subject: reftest_compare_surfaces: Report how much the images differ
 
-Some of the reftests don't produce *identical* results on all
-architectures, but do produce results that are visually
-indistinguishable. Report how many pixels differ and by how much, so we
-can get an idea of what's a rounding error and what's a serious problem.
+In unattended/non-interactive/autobuilder environments where the images
+are not trivially accessible, this provides a way to distinguish between
+totally different rendering and more subtle issues.
 
 Signed-off-by: Simon McVittie <smcv@debian.org>
 Forwarded: https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3195
index 1fd7144cab299d1b34c16d2a64472bfe071f1221..070a1c7bcd7bfbca997dbb501fc7476bd5cf9357 100644 (file)
@@ -9,18 +9,10 @@ Each .ui or .node reftest can have an accompanying .keyfile file
 like this:
 
     [reftest]
-    accepted-diff-level=2
-    accepted-diff-pixels=100
     tolerated-diff-level=20
     tolerated-diff-pixels=1000
 
-If the number of pixels that differ from the reference is no more than
-accepted-diff-pixels, and each channel in each of those pixels differs
-from the reference by no more than accepted-diff-level, then we consider
-that to be a full success, and don't even save the .diff.png for
-analysis.
-
-If that check fails, but the number of pixels that differ is no more
+If the image differs, but the number of pixels that differ is no more
 than tolerated-diff-pixels and the differences are no more than
 tolerated-diff-level, then we treat it as a success with warnings, save
 the .diff.png for analysis, and use g_test_incomplete() to record the
@@ -30,13 +22,13 @@ Signed-off-by: Simon McVittie <smcv@debian.org>
 Forwarded: https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3195
 Applied-upstream: no, upstream want reftests to be a strict pass/fail with identical results required
 ---
- testsuite/gsk/compare-render.c     | 43 ++++++++++++++++++++++++++++++++++++--
- testsuite/reftests/gtk-reftest.c   | 43 ++++++++++++++++++++++++++++++++++++--
+ testsuite/gsk/compare-render.c     | 31 ++++++++++++++++++++++++++++++-
+ testsuite/reftests/gtk-reftest.c   | 32 +++++++++++++++++++++++++++++++-
  testsuite/reftests/image-compare.c |  2 +-
- 3 files changed, 83 insertions(+), 5 deletions(-)
+ 3 files changed, 62 insertions(+), 3 deletions(-)
 
 diff --git a/testsuite/gsk/compare-render.c b/testsuite/gsk/compare-render.c
-index da6f9e2..a4afd75 100644
+index da6f9e2..237b8c4 100644
 --- a/testsuite/gsk/compare-render.c
 +++ b/testsuite/gsk/compare-render.c
 @@ -98,6 +98,12 @@ get_output_file (const char *file,
@@ -52,7 +44,7 @@ index da6f9e2..a4afd75 100644
  static void
  save_image (cairo_surface_t *surface,
              const char      *test_name,
-@@ -242,12 +248,45 @@ main (int argc, char **argv)
+@@ -242,12 +248,35 @@ main (int argc, char **argv)
  
        if (diff_surface)
          {
@@ -60,18 +52,12 @@ index da6f9e2..a4afd75 100644
 +          GKeyFile *keyfile = g_key_file_new ();
 +          guint64 tolerated_diff = 0;
 +          guint64 tolerated_pixels = 0;
-+          guint64 accepted_diff = 0;
-+          guint64 accepted_pixels = 0;
 +
 +          if (keyfile_path != NULL && g_file_test (keyfile_path, G_FILE_TEST_EXISTS))
 +            {
 +              GError *error = NULL;
 +              g_key_file_load_from_file (keyfile, keyfile_path, G_KEY_FILE_NONE, &error);
 +              g_assert_no_error (error);
-+              accepted_diff = g_key_file_get_uint64 (keyfile, "reftest", "accepted-diff-level", NULL);
-+              g_print ("Maximum difference accepted: %" G_GUINT64_FORMAT " levels\n", accepted_diff);
-+              accepted_pixels = g_key_file_get_uint64 (keyfile, "reftest", "accepted-diff-pixels", NULL);
-+              g_print ("Different pixels accepted: %" G_GUINT64_FORMAT "\n", accepted_pixels);
 +              tolerated_diff = g_key_file_get_uint64 (keyfile, "reftest", "tolerated-diff-level", NULL);
 +              g_print ("Maximum difference tolerated: %" G_GUINT64_FORMAT " levels\n", tolerated_diff);
 +              tolerated_pixels = g_key_file_get_uint64 (keyfile, "reftest", "tolerated-diff-pixels", NULL);
@@ -81,17 +67,12 @@ index da6f9e2..a4afd75 100644
            g_print ("%u (out of %u) pixels differ from reference by up to %u levels\n",
                     pixels_changed, pixels, max_diff);
  
--          save_image (diff_surface, node_file, ".diff.png");
-+          if (max_diff > accepted_diff || pixels_changed > accepted_pixels)
-+            save_image (diff_surface, node_file, ".diff.png");
-+
+           save_image (diff_surface, node_file, ".diff.png");
            cairo_surface_destroy (diff_surface);
 -          success = FALSE;
 +
-+          if (max_diff <= accepted_diff && pixels_changed <= accepted_pixels)
-+            g_print ("differences are within acceptable range\n");
-+          else if (max_diff <= tolerated_diff && pixels_changed <= tolerated_pixels)
-+            g_print ("not right, but close enough\n");
++          if (max_diff <= tolerated_diff && pixels_changed <= tolerated_pixels)
++            g_print ("not right, but close enough?\n");
 +          else
 +            g_test_fail ();
 +
@@ -101,7 +82,7 @@ index da6f9e2..a4afd75 100644
      }
  
 diff --git a/testsuite/reftests/gtk-reftest.c b/testsuite/reftests/gtk-reftest.c
-index 6ef17aa..791e95c 100644
+index 6ef17aa..3ef6d32 100644
 --- a/testsuite/reftests/gtk-reftest.c
 +++ b/testsuite/reftests/gtk-reftest.c
 @@ -290,6 +290,12 @@ save_image (cairo_surface_t *surface,
@@ -117,7 +98,7 @@ index 6ef17aa..791e95c 100644
  static void
  test_ui_file (GFile *file)
  {
-@@ -326,10 +332,43 @@ test_ui_file (GFile *file)
+@@ -326,10 +332,34 @@ test_ui_file (GFile *file)
  
    if (diff_image)
      {
@@ -125,18 +106,12 @@ index 6ef17aa..791e95c 100644
 +      GKeyFile *keyfile = g_key_file_new ();
 +      guint64 tolerated_diff = 0;
 +      guint64 tolerated_pixels = 0;
-+      guint64 accepted_diff = 0;
-+      guint64 accepted_pixels = 0;
 +
 +      if (keyfile_path != NULL)
 +        {
 +          GError *error = NULL;
 +          g_key_file_load_from_file (keyfile, keyfile_path, G_KEY_FILE_NONE, &error);
 +          g_assert_no_error (error);
-+          accepted_diff = g_key_file_get_uint64 (keyfile, "reftest", "accepted-diff-level", NULL);
-+          g_test_message ("Maximum difference accepted: %" G_GUINT64_FORMAT " levels", accepted_diff);
-+          accepted_pixels = g_key_file_get_uint64 (keyfile, "reftest", "accepted-diff-pixels", NULL);
-+          g_test_message ("Different pixels accepted: %" G_GUINT64_FORMAT, accepted_pixels);
 +          tolerated_diff = g_key_file_get_uint64 (keyfile, "reftest", "tolerated-diff-level", NULL);
 +          g_test_message ("Maximum difference tolerated: %" G_GUINT64_FORMAT " levels", tolerated_diff);
 +          tolerated_pixels = g_key_file_get_uint64 (keyfile, "reftest", "tolerated-diff-pixels", NULL);
@@ -145,16 +120,12 @@ index 6ef17aa..791e95c 100644
 +
        g_test_message ("%u (out of %u) pixels differ from reference by up to %u levels",
                        pixels_changed, pixels, max_diff);
--      save_image (diff_image, ui_file, ".diff.png");
--      g_test_fail ();
 +
-+      if (max_diff > accepted_diff || pixels_changed > accepted_pixels)
-+        save_image (diff_image, ui_file, ".diff.png");
+       save_image (diff_image, ui_file, ".diff.png");
+-      g_test_fail ();
 +
-+      if (max_diff <= accepted_diff && pixels_changed <= accepted_pixels)
-+        g_test_message ("differences are within acceptable range");
-+      else if (max_diff <= tolerated_diff && pixels_changed <= tolerated_pixels)
-+        g_test_incomplete ("not right, but close enough");
++      if (max_diff <= tolerated_diff && pixels_changed <= tolerated_pixels)
++        g_test_incomplete ("not right, but close enough?");
 +      else
 +        g_test_fail ();
 +